[lldb][test] Combine libstdc++ and libc++ std::variant tests into generic test #147253
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
@llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) ChangesThis combines the libc++ and libstdc++ test cases. The libstdc++ test had an additional test-case for "reference to typedef". So I added those to the generic test. The rest of the tests was the same as libc++. Split out from #146740 Full diff: https://github.com/llvm/llvm-project/pull/147253.diff 6 Files Affected:
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/Makefile b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/variant/Makefile
similarity index 82%
rename from lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/Makefile
rename to lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/variant/Makefile
index 7eeff7407804d..d5f5fec8441b5 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/Makefile
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/variant/Makefile
@@ -1,6 +1,4 @@
CXX_SOURCES := main.cpp
-
-USE_LIBCPP := 1
-
CXXFLAGS_EXTRAS := -std=c++17
+
include Makefile.rules
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/variant/TestDataFormatterStdVariant.py
similarity index 53%
rename from lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
rename to lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/variant/TestDataFormatterStdVariant.py
index 47e07a5ce3f5b..cf2569b02a586 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/TestDataFormatterLibcxxVariant.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/variant/TestDataFormatterStdVariant.py
@@ -2,53 +2,31 @@
Test lldb data formatter subsystem.
"""
-
import lldb
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
from lldbsuite.test import lldbutil
-class LibcxxVariantDataFormatterTestCase(TestBase):
- @add_test_categories(["libc++"])
- ## Clang 7.0 is the oldest Clang that can reliably parse newer libc++ versions
- ## with -std=c++17.
- @skipIf(
- oslist=no_match(["macosx"]), compiler="clang", compiler_version=["<", "7.0"]
- )
- ## We are skipping gcc version less that 5.1 since this test requires -std=c++17
- @skipIf(compiler="gcc", compiler_version=["<", "5.1"])
- ## std::get is unavailable for std::variant before macOS 10.14
- @skipIf(macos_version=["<", "10.14"])
- def test_with_run_command(self):
+class StdVariantDataFormatterTestCase(TestBase):
+ def do_test(self):
"""Test that that file and class static variables display correctly."""
- self.build()
(self.target, self.process, _, bkpt) = lldbutil.run_to_source_breakpoint(
self, "// break here", lldb.SBFileSpec("main.cpp", False)
)
- self.runCmd("frame variable has_variant")
-
- output = self.res.GetOutput()
-
- ## The variable has_variant tells us if the test program
- ## detected we have a sufficient libc++ version to support variant
- ## false means we do not and therefore should skip the test
- if output.find("(bool) has_variant = false") != -1:
- self.skipTest("std::variant not supported")
-
- lldbutil.continue_to_breakpoint(self.process, bkpt)
-
- self.expect(
- "frame variable v1",
- substrs=["v1 = Active Type = int {", "Value = 12", "}"],
- )
+ for name in ["v1", "v1_typedef"]:
+ self.expect(
+ "frame variable " + name,
+ substrs=[name + " = Active Type = int {", "Value = 12", "}"],
+ )
- self.expect(
- "frame variable v1_ref",
- patterns=["v1_ref = 0x.* Active Type = int : {", "Value = 12", "}"],
- )
+ for name in ["v1_ref", "v1_typedef_ref"]:
+ self.expect(
+ "frame variable " + name,
+ patterns=[name + " = 0x.* Active Type = int : {", "Value = 12", "}"],
+ )
self.expect(
"frame variable v_v1",
@@ -86,3 +64,13 @@ def test_with_run_command(self):
"frame variable v_300_types_no_value",
substrs=["v_300_types_no_value = No Value"],
)
+
+ @add_test_categories(["libc++"])
+ def test_libcxx(self):
+ self.build(dictionary={"USE_LIBCPP": 1})
+ self.do_test()
+
+ @add_test_categories(["libstdcxx"])
+ def test_libstdcxx(self):
+ self.build(dictionary={"USE_LIBSTDCPP": 1})
+ self.do_test()
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/variant/main.cpp
similarity index 85%
rename from lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/main.cpp
rename to lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/variant/main.cpp
index 36e0f74f831f8..909dafda72213 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/main.cpp
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/variant/main.cpp
@@ -8,12 +8,9 @@ struct S {
};
int main() {
- bool has_variant = true;
-
- printf("%d\n", has_variant); // break here
-
std::variant<int, double, char> v1;
std::variant<int, double, char> &v1_ref = v1;
+
using V1_typedef = std::variant<int, double, char>;
V1_typedef v1_typedef;
V1_typedef &v1_typedef_ref = v1_typedef;
@@ -22,7 +19,7 @@ int main() {
std::variant<int, double, char> v3;
std::variant<std::variant<int, double, char>> v_v1;
std::variant<int, double, char> v_no_value;
- // The next variant has many types, meaning the type index does not fit in
+ // The next variant has 300 types, meaning the type index does not fit in
// a byte and must be `unsigned short` instead of `unsigned char` when
// using the unstable libc++ ABI. With stable libc++ ABI, the type index
// is always just `unsigned int`.
@@ -43,11 +40,13 @@ int main() {
int, int, int, int, int, int, int, int, int, int, int, int, int, int, int,
int, int, int, int, int, int, int, int, int, int, int, int, int, int, int,
int, int, int, int, int, int, int, int, int, int, int, int, int, int, int,
- int, int, int, int, int, int, int, int, int, int, int, int>
- v_many_types_no_value;
+ int, int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+ int, int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+ int, int, int, int, int, int, int, int, int, int, int, int, int, int, int,
+ int, int, int, int, int, int, int, int, int, int, int, int, int, int, int>
+ v_300_types_no_value;
v1 = 12; // v contains int
- v1_typedef = v1;
v_v1 = v1;
int i = std::get<int>(v1);
printf("%d\n", i); // break here
@@ -74,11 +73,11 @@ int main() {
printf("%zu\n", v_no_value.index());
try {
- v_many_types_no_value.emplace<0>(S());
+ v_300_types_no_value.emplace<0>(S());
} catch (...) {
}
- printf("%zu\n", v_many_types_no_value.index());
+ printf("%zu\n", v_300_types_no_value.index());
return 0; // break here
}
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp
deleted file mode 100644
index 560ec692f30ed..0000000000000
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/variant/main.cpp
+++ /dev/null
@@ -1,95 +0,0 @@
-#include <cstdio>
-#include <string>
-#include <vector>
-
-// If we have libc++ 4.0 or greater we should have <variant>
-// According to libc++ C++1z status page https://libcxx.llvm.org/cxx1z_status.html
-#if _LIBCPP_VERSION >= 4000
-#include <variant>
-#define HAVE_VARIANT 1
-#else
-#define HAVE_VARIANT 0
-#endif
-
-struct S {
- operator int() { throw 42; }
-} ;
-
-
-int main()
-{
- bool has_variant = HAVE_VARIANT ;
-
- printf( "%d\n", has_variant ) ; // break here
-
-#if HAVE_VARIANT == 1
- std::variant<int, double, char> v1;
- std::variant<int, double, char> &v1_ref = v1;
- std::variant<int, double, char> v2;
- std::variant<int, double, char> v3;
- std::variant<std::variant<int,double,char>> v_v1 ;
- std::variant<int, double, char> v_no_value;
- // The next variant has 300 types, meaning the type index does not fit in
- // a byte and must be `unsigned short` instead of `unsigned char` when
- // using the unstable libc++ ABI. With stable libc++ ABI, the type index
- // is always just `unsigned int`.
- std::variant<
- int, int, int, int, int, int, int, int, int, int, int, int, int, int,
- int, int, int, int, int, int, int, int, int, int, int, int, int, int,
- int, int, int, int, int, int, int, int, int, int, int, int, int, int,
- int, int, int, int, int, int, int, int, int, int, int, int, int, int,
- int, int, int, int, int, int, int, int, int, int, int, int, int, int,
- int, int, int, int, int, int, int, int, int, int, int, int, int, int,
- int, int, int, int, int, int, int, int, int, int, int, int, int, int,
- int, int, int, int, int, int, int, int, int, int, int, int, int, int,
- int, int, int, int, int, int, int, int, int, int, int, int, int, int,
- int, int, int, int, int, int, int, int, int, int, int, int, int, int,
- int, int, int, int, int, int, int, int, int, int, int, int, int, int,
- int, int, int, int, int, int, int, int, int, int, int, int, int, int,
- int, int, int, int, int, int, int, int, int, int, int, int, int, int,
- int, int, int, int, int, int, int, int, int, int, int, int, int, int,
- int, int, int, int, int, int, int, int, int, int, int, int, int, int,
- int, int, int, int, int, int, int, int, int, int, int, int, int, int,
- int, int, int, int, int, int, int, int, int, int, int, int, int, int,
- int, int, int, int, int, int, int, int, int, int, int, int, int, int,
- int, int, int, int, int, int, int, int, int, int, int, int, int, int,
- int, int, int, int, int, int, int, int, int, int, int, int, int, int,
- int, int, int, int, int, int, int, int, int, int, int, int, int, int,
- int, int, int, int, int, int>
- v_300_types_no_value;
-
- v1 = 12; // v contains int
- v_v1 = v1 ;
- int i = std::get<int>(v1);
- printf( "%d\n", i ); // break here
-
- v2 = 2.0 ;
- double d = std::get<double>(v2) ;
- printf( "%f\n", d );
-
- v3 = 'A' ;
- char c = std::get<char>(v3) ;
- printf( "%d\n", c );
-
- // Checking v1 above and here to make sure we done maintain the incorrect
- // state when we change its value.
- v1 = 2.0;
- d = std::get<double>(v1) ;
- printf( "%f\n", d ); // break here
-
- try {
- v_no_value.emplace<0>(S());
- } catch( ... ) {}
-
- printf( "%zu\n", v_no_value.index() ) ;
-
- try {
- v_300_types_no_value.emplace<0>(S());
- } catch (...) {
- }
-
- printf("%zu\n", v_300_types_no_value.index());
-#endif
-
- return 0; // break here
-}
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/Makefile b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/Makefile
deleted file mode 100644
index 104f82809c7a3..0000000000000
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/Makefile
+++ /dev/null
@@ -1,5 +0,0 @@
-CXX_SOURCES := main.cpp
-
-USE_LIBSTDCPP := 1
-CXXFLAGS_EXTRAS := -std=c++17
-include Makefile.rules
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/TestDataFormatterLibStdcxxVariant.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/TestDataFormatterLibStdcxxVariant.py
deleted file mode 100644
index 394e221809f7c..0000000000000
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/TestDataFormatterLibStdcxxVariant.py
+++ /dev/null
@@ -1,101 +0,0 @@
-"""
-Test lldb data formatter for LibStdC++ std::variant.
-"""
-
-
-import lldb
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-
-
-class LibStdcxxVariantDataFormatterTestCase(TestBase):
- @add_test_categories(["libstdcxx"])
- def test_with_run_command(self):
- """Test LibStdC++ std::variant data formatter works correctly."""
- self.build()
-
- (self.target, self.process, _, bkpt) = lldbutil.run_to_source_breakpoint(
- self, "// break here", lldb.SBFileSpec("main.cpp", False)
- )
-
- lldbutil.continue_to_breakpoint(self.process, bkpt)
-
- for name in ["v1", "v1_typedef"]:
- self.expect(
- "frame variable " + name,
- substrs=[name + " = Active Type = int {", "Value = 12", "}"],
- )
-
- for name in ["v1_ref", "v1_typedef_ref"]:
- self.expect(
- "frame variable " + name,
- patterns=[name + " = 0x.* Active Type = int : {", "Value = 12", "}"],
- )
-
- self.expect(
- "frame variable v_v1",
- substrs=[
- "v_v1 = Active Type = std::variant<int, double, char> {",
- "Value = Active Type = int {",
- "Value = 12",
- "}",
- "}",
- ],
- )
-
- lldbutil.continue_to_breakpoint(self.process, bkpt)
-
- self.expect(
- "frame variable v1",
- substrs=["v1 = Active Type = double {", "Value = 2", "}"],
- )
-
- lldbutil.continue_to_breakpoint(self.process, bkpt)
-
- self.expect(
- "frame variable v2",
- substrs=["v2 = Active Type = double {", "Value = 2", "}"],
- )
-
- self.expect(
- "frame variable v3",
- substrs=["v3 = Active Type = char {", "Value = 'A'", "}"],
- )
- """
- TODO: temporarily disable No Value tests as they seem to fail on ubuntu/debian
- bots. Pending reproduce and investigation.
-
- self.expect("frame variable v_no_value", substrs=["v_no_value = No Value"])
-
- self.expect(
- "frame variable v_many_types_no_value",
- substrs=["v_many_types_no_value = No Value"],
- )
- """
-
- @add_test_categories(["libstdcxx"])
- def test_invalid_variant_index(self):
- """Test LibStdC++ data formatter for std::variant with invalid index."""
- self.build()
-
- (self.target, self.process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
- self, "// break here", lldb.SBFileSpec("main.cpp", False)
- )
-
- lldbutil.continue_to_breakpoint(self.process, bkpt)
-
- self.expect(
- "frame variable v1",
- substrs=["v1 = Active Type = int {", "Value = 12", "}"],
- )
-
- var_v1 = thread.frames[0].FindVariable("v1")
- var_v1_raw_obj = var_v1.GetNonSyntheticValue()
- index_obj = var_v1_raw_obj.GetChildMemberWithName("_M_index")
- self.assertTrue(index_obj and index_obj.IsValid())
-
- INVALID_INDEX = "100"
- index_obj.SetValueFromCString(INVALID_INDEX)
-
- self.expect("frame variable v1", substrs=["v1 = <Invalid>"])
|
Michael137
force-pushed
the
lldb/consolidate-stl-tests-variant
branch
from
5ec10f9 to
a6ddbc5
Compare
Michael137
added a commit
to Michael137/llvm-project
that referenced
this pull request
Jul 7, 2025
Instead of using the byte-size to make a guess at what the `std::variant_npos` value is, just look it up in debug-info. Unblocks llvm#147253
Michael137
added a commit
that referenced
this pull request
Jul 7, 2025
… variants (#147283) A default-constructed variant has a valid index (being the first element of the variant). The only case where the index is variant_npos is when the variant is "valueless", which [according to cppreference](https://en.cppreference.com/w/cpp/utility/variant/valueless_by_exception.html) only happens when an exception is thrown during assignment to the variant. I adjusted the test to test that scenario, and the formatter seems to work. Unblocks #147253
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jul 7, 2025
…r valueless variants (#147283) A default-constructed variant has a valid index (being the first element of the variant). The only case where the index is variant_npos is when the variant is "valueless", which [according to cppreference](https://en.cppreference.com/w/cpp/utility/variant/valueless_by_exception.html) only happens when an exception is thrown during assignment to the variant. I adjusted the test to test that scenario, and the formatter seems to work. Unblocks llvm/llvm-project#147253
Michael137
force-pushed
the
lldb/consolidate-stl-tests-variant
branch
from
a6ddbc5 to
3cf500f
Compare
Michael137
force-pushed
the
lldb/consolidate-stl-tests-variant
branch
from
3cf500f to
d56385c
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Member
Author
|
Confused why the variant tests fail for libstdc++ (they don't locally) |
Member
Author
|
Hah i can repro it when i run both libcxx and libstdcxx tests together (previously i just ran with |
Member
Author
|
Yea I can repro this if I use the same debugger instance to debug a target compiled against libc++, then print the typedef, and then debug a libstdc++ target. I think the issue is that the formatter infra caches Same reason as #110767 |
Member
Author
|
For now i think I'll probably i'll reset the formatters in the teardown hook, but @jimingham comment on #110767 might be a good follow-up:
|
labath
approved these changes
Jul 8, 2025
Collaborator
labath
left a comment
labath
left a comment
There was a problem hiding this comment.
This looks good. The problem with caching is not related to this patch.
For now i think I'll probably i'll reset the formatters in the teardown hook, but @jimingham comment on #110767 might be a good follow-up:
Since this is just an optimization, we should first prove to ourselves that failing to put one of these type names in the exact matches really makes enough difference to bother with a complex solution. If the gains are some but not huge, some boneheaded solution like never caching anything where the from type has a '<' and a '>' is probably sufficient.
However, I don't see how templates are related to this. I think the same thing can happen if I have using MyType = TypeA; in one binary and using MyType = TypeB; in another. If I cache the formatter for TypeA as the formatter for MyType (because that's correct for one binary) and then use that in the other binary, things will blow up. Technically, even TypeA in one binary need not have anything to do with TypeA in the second one, but we currently don't have a way to say which version of TypeA we are trying to format.
I think my ideal solution would be to have the formatter cache be per-target. I like that because it opens the door to having data formatters be registered per-target, which then means you don't have to have uber-formatters which know how to format every version of some struct.
Member
Author
Yup agreed, it's unrelated to templates. Re. Jim's comment, I meant the part about checking whether caching typedefs at all is a good idea. But making the cache per-target as you suggest seems like a good solution. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This combines the libc++ and libstdc++ test cases. The libstdc++ test had an additional test-case for "reference to typedef". So I added those to the generic test. The rest of the tests was the same as libc++.
I also moved the test-case for checking invalid variant indexes into a separate libstdcpp test because it relied on the layout of the libstdc++ type. We should probably rewrite it in the "simulator" style. But for now I just moved it.
This also removes some redundant checks for libc++ versions and existence of the
variantheader, which at this point should be available anywhere these tests are run.Split out from #146740